Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bring back optional field test #1067

Merged
merged 3 commits into from
Oct 9, 2024
Merged

bring back optional field test #1067

merged 3 commits into from
Oct 9, 2024

Conversation

bobzhang
Copy link
Contributor

@bobzhang bobzhang commented Oct 8, 2024

No description provided.

Copy link

peter-jerry-ye-code-review bot commented Oct 8, 2024

Observed Problems from git diff

  1. Incorrect Type Suffixes in bool.mbt:

    • The suffixes used for integer types (like L, U, UL) are incorrect. In MoonBit, these should be I64, U, U64 respectively.
    • Suggested Fix: Replace 1L with 1I64, 1U with 1U, and 1UL with 1U64.
  2. Incorrect JSON Boolean Values in json.mbt:

    • The JSON boolean values True and False should be true and false respectively in MoonBit.
    • Suggested Fix: Replace True with true and False with false.
  3. Inconsistent Test Naming and Inspection in to_json_test.mbt:

    • The test comment "optinal field" is a typo and should be "optional field".
    • The inspect_double_array! macro seems to be replaced with a more appropriate inspect! macro in the rational_test.mbt file.
    • Suggested Fix: Rename the test to "optional field" and ensure consistency in using the inspect! macro for array inspection.

These changes should align the code with the MoonBit language specifications and improve code readability and correctness.

@coveralls
Copy link
Collaborator

coveralls commented Oct 8, 2024

Pull Request Test Coverage Report for Build 3349

Details

  • 6 of 8 (75.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 83.256%

Changes Missing Coverage Covered Lines Changed/Added Lines %
builtin/json.mbt 0 2 0.0%
Totals Coverage Status
Change from base Build 3347: 0.5%
Covered Lines: 4102
Relevant Lines: 4927

💛 - Coveralls

json/to_json_test.mbt Outdated Show resolved Hide resolved
@bobzhang bobzhang merged commit bee9515 into main Oct 9, 2024
12 checks passed
@bobzhang bobzhang deleted the hongbo/bringback_json_test branch October 9, 2024 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants